Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ethernet: Add support stm32n6570_dk #87562

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Ethernet: Add support stm32n6570_dk #87562

wants to merge 2 commits into from
+80 −8

Conversation

marwaiehm-st
Copy link
Collaborator

@marwaiehm-st marwaiehm-st commented Mar 24, 2025 •
edited
Loading

Add ethernet node of stm32n6570_dk
Integrate GRMII and RMII interfaces

Based on this PR: #87593

@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch 3 times, most recently from 264dc7a to 091627e Compare March 24, 2025 13:32
@maass-hamburg
Copy link
Collaborator

maass-hamburg commented Mar 24, 2025 •
edited
Loading

instead of using and extending the internal eth_phy_get_link_state(), maybe use the phy_get_link_state() or even phy_link_callback_set() from the ethernet phy api. The later also has the benefit, that autonegation can be done on every change of the link not just on startup. the internal eth_phy_get_link_state() could still be used for the one that don't have a st,stm32-mdio

@maass-hamburg
Copy link
Collaborator

the st,stm32-mdio also uses only HAL_ETH_ReadPHYRegister() and HAL_ETH_WritePHYRegister() internally, like eth_phy_get_link_state(). That means, that eth_phy_get_link_state() can be fully replaced by the use of the ethernet phy api. It just have to make sure, that all soc, that select ETH_STM32_HAL_API_V2 also get the st,stm32-mdio dev. And all boards the also the generic ethernet-phy.

@maass-hamburg
Copy link
Collaborator

started something like I described there: #87593

@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch 2 times, most recently from ee39d6c to 6d119f4 Compare March 26, 2025 14:44
@marwaiehm-st marwaiehm-st marked this pull request as ready for review March 26, 2025 15:56
@marwaiehm-st marwaiehm-st added the DNM This PR should not be merged (Do Not Merge) label Mar 26, 2025
@pillo79 pillo79 assigned erwango and unassigned pillo79 Mar 27, 2025
@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch from 6d119f4 to 3656cfe Compare March 27, 2025 14:33
@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch 3 times, most recently from 3238878 to 3197859 Compare March 27, 2025 15:05
@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch from 3197859 to a016e40 Compare March 28, 2025 13:29
@github-actions github-actions bot added the Release Notes To be mentioned in the release notes label Mar 28, 2025
@maass-hamburg
Copy link
Collaborator

@marwaiehm-st please rebase to current main, now that #87593 is merged

@marwaiehm-st
boards: st: stm32n6570_dk: Add ethernet node
e5a4f9d 
Add the Ethernet MAC and MDIO nodes in the device tree.
Add Kconfig for Ethernet Support.

Signed-off-by: IBEN EL HADJ MESSAOUD Marwa <marwa.ibenelhadjmessaoud-ext@st.com>
@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch from a016e40 to 51bdac7 Compare April 2, 2025 08:57
@marwaiehm-st marwaiehm-st removed the DNM This PR should not be merged (Do Not Merge) label Apr 2, 2025
erwango
erwango reviewed Apr 2, 2025
View reviewed changes
drivers/ethernet/eth_stm32_hal.c Outdated
Comment on lines 84 to 89
#define STM32_ETH_PHY_MODE(node_id) \
((DT_ENUM_HAS_VALUE(node_id, phy_connection_type, mii) ? HAL_ETH_MII_MODE : \
(DT_ENUM_HAS_VALUE(node_id, phy_connection_type, rmii) ? HAL_ETH_RMII_MODE : \
(DT_ENUM_HAS_VALUE(node_id, phy_connection_type, gmii) ? HAL_ETH_GMII_MODE : \
(DT_ENUM_HAS_VALUE(node_id, phy_connection_type, rgmii) ? HAL_ETH_RGMII_MODE : \
HAL_ETH_RMII_MODE)))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use ETH_MEDIA_INTERFACE_FOO values now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used ETH_MEDIA_INTERFACE_FOO to support the api v1, but here we have only stm32n6.
We can add the definition of ETH_MEDIA_INTERFACE_FOO for RGMII and GMII, if it makes the code compatible

#define ETH_MEDIA_INTERFACE_RGMII		HAL_ETH_RGMII_MODE
#define ETH_MEDIA_INTERFACE_GMII	        HAL_ETH_GMII_MODE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be cleaner indeed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we remove the ETH_MEDIA_INTERFACE_FOO, but we should be able to use only one macro version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the definition of ETH_MEDIA_INTERFACE_FOO and using directly HAL_ETH_XMII for API V2, but for API V1 we can only use ETH_MEDIA_INTERFACE_FOO .

In case of removing the definition "ETH_MEDIA_INTERFACE_FOO", we should add definition for HAL_ETH_XMII_MODE to support API v1.

#if !defined(CONFIG_ETH_STM32_HAL_API_V2)
#define HAL_ETH_MII_MODE   ETH_MEDIA_INTERFACE_MII
#define HAL_ETH_RMII_MODE ETH_MEDIA_INTERFACE_RMII
#endif

#define STM32_ETH_PHY_MODE(node_id) \
	(DT_ENUM_HAS_VALUE(node_id, phy_connection_type, mii) ? \
		HAL_ETH_MII_MODE : HAL_ETH_RMII_MODE)

@marwaiehm-st marwaiehm-st requested a review from erwango April 2, 2025 12:10
@decsny decsny removed their request for review April 2, 2025 15:57
maass-hamburg
maass-hamburg reviewed Apr 3, 2025
View reviewed changes
drivers/ethernet/eth_stm32_hal.c Outdated
@@ -1477,7 +1493,9 @@ static const struct eth_stm32_hal_dev_cfg eth0_config = {
};

BUILD_ASSERT(DT_ENUM_HAS_VALUE(MAC_NODE, phy_connection_type, mii) ||
DT_ENUM_HAS_VALUE(MAC_NODE, phy_connection_type, rmii),
DT_ENUM_HAS_VALUE(MAC_NODE, phy_connection_type, rmii) ||
DT_ENUM_HAS_VALUE(MAC_NODE, phy_connection_type, rgmii) ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use a IF_ENABLED(DT_HAS_COMPAT_STATUS_OKAY(st_stm32n6_ethernet),(check for rgmii and gmii)), to only allow gigabit connection types on supported soc

erwango
erwango reviewed Apr 3, 2025
View reviewed changes
drivers/ethernet/eth_stm32_hal.c Outdated
Comment on lines 1153 to 1155
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32n6_ethernet)
PHY_LINK_IS_SPEED_1000M(state->speed) ? ETH_SPEED_1000M :
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can IF_ENABLED(DT_HAS_COMPAT_STATUS_OKAY(st_stm32n6_ethernet) could also be used here ?

@marwaiehm-st
drivers: ethernet: Integrate GRMII and RMII interfaces
44f863d 
- Added macros `STM32_ETH_PHY_MODE`
  to determine the PHY mode (RGMII, GMII, RMII, and MII)
  from the `phy_connection_type` property in the device tree.
- Removed previous definitions for ETH_MEDIA_INTERFACE_MII
  and ETH_MEDIA_INTERFACE_RMII.
- Updated STM32_ETH_PHY_MODE macro to use ETH_MII_MODE
  and ETH_RMII_MODE.

Signed-off-by: IBEN EL HADJ MESSAOUD Marwa <marwa.ibenelhadjmessaoud-ext@st.com>
@marwaiehm-st marwaiehm-st force-pushed the upstream_eth_disco_n6 branch from 09b9496 to 44f863d Compare April 3, 2025 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erwango erwango erwango left review comments

@maass-hamburg maass-hamburg maass-hamburg left review comments

@djiatsaf-st djiatsaf-st Awaiting requested review from djiatsaf-st

@facchinm facchinm Awaiting requested review from facchinm

@FRASTM FRASTM Awaiting requested review from FRASTM

@GeorgeCGV GeorgeCGV Awaiting requested review from GeorgeCGV

@lmajewski lmajewski Awaiting requested review from lmajewski

@mathieuchopstm mathieuchopstm Awaiting requested review from mathieuchopstm

@pdgendt pdgendt Awaiting requested review from pdgendt

@pillo79 pillo79 Awaiting requested review from pillo79

@fabiobaltieri fabiobaltieri Awaiting requested review from fabiobaltieri

@kartben kartben Awaiting requested review from kartben

At least 2 approving reviews are required to merge this pull request.

Assignees

@erwango erwango

Labels
area: Ethernet area: MDIO platform: STM32 ST Micro STM32 Release Notes To be mentioned in the release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marwaiehm-st @maass-hamburg @erwango @pillo79